Skip to content

[이준기] week8, 11#336

Open
eonpain wants to merge 16 commits intocodeit-bootcamp-frontend:part2-이준기from
eonpain:part2-이준기-week8

Hidden character warning

The head ref may contain hidden characters: "part2-\uc774\uc900\uae30-week8"
Open

[이준기] week8, 11#336
eonpain wants to merge 16 commits intocodeit-bootcamp-frontend:part2-이준기from
eonpain:part2-이준기-week8

Conversation

@eonpain
Copy link
Copy Markdown
Collaborator

@eonpain eonpain commented Jan 2, 2024

요구사항

8주차

기본

  • 체크리스트 1(링크를 입력하고 “추가하기” 버튼을 누르면 “폴더에 추가” 모달이 보이나요?)
  • 체크리스트 2("폴더 추가” 버튼을 누르면 “폴더 추가” 모달이 보이나요?)
  • 체크리스트 3("공유” 버튼을 누르면 “폴더 공유” 모달이 보이나요?)
  • 체크리스트 4(“이름 변경” 버튼을 누르면 “폴더 이름 변경” 모달이 보이나요?)
  • 체크리스트 5(“삭제” 버튼을 누르면 “폴더 삭제” 모달이 보이나요?)
  • 체크리스트 6(케밥 버튼을 누르면 “삭제하기”, “폴더에 추가” 버튼이 있는 팝오버가 보이나요?)
  • 체크리스트 7(팝오버에 있는 “삭제하기” 버튼을 누르면 “링크 삭제” 모달이 보이나요?)
  • 체크리스트 8(팝오버에 있는 “폴더에 추가” 버튼을 누르면 “폴더에 추가” 모달이 보이나요?)
  • 체크리스트 9(“폴더 공유” 모달에서 “카카오톡 아이콘”을 클릭하면 카카오로 공유 폴더 페이지 링크 공유하기 가능한가요?)
  • 체크리스트 10(“폴더 공유” 모달에서 “페이스북 아이콘”을 클릭하면 페이스북으로 공유 폴더 페이지 링크 공유하기 가능한가요?)
  • 체크리스트 11(“폴더 공유” 모달에서 “링크 아이콘”을 클릭하면 클립보드에 공유 폴더 페이지 링크가 복사 되나요?)

11주차

기본

  • 체크리스트 1(TypeScript를 활용해 프로젝트에 타입 명시가 필요한 곳을 찾아 타입을 명시했나요?)
  • 체크리스트 2(링크 검색바에 검색어를 입력하면 현재 폴더에 있는 링크들 중 “url”, “title”, “description” 중 하나에 검색어가 포함된 링크들만 필터된 상태로 볼 수 있나요?)
  • 체크리스트 3(링크 검색바에 입력 값이 있는 경우 x 버튼이 보이나요?)

심화

  • 체크리스트 1(상단에 있던 링크 추가하기 영역이 가려져 보이지 않을 때 최하단에 링크 추가하기 영역을 고정하도록 만들었나요?)
  • 체크리스트 2(푸터가 시작되는 지점에서는 최하단에 고정된 링크 추가하기 영역이 보이지 않도록 했나요?)

주요 변경사항

스크린샷

image

멘토에게

  • 이번 미션때 vite를 사용하지 않았는데 꼭 써야하나요? 그래야 한다면 이유가 궁금합니다.
  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@eonpain eonpain requested a review from Tanney-102 January 2, 2024 07:43
@eonpain eonpain added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성 죄송합니다... labels Jan 2, 2024
Copy link
Copy Markdown
Collaborator

@Tanney-102 Tanney-102 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번 미션때 vite를 사용하지 않았는데 꼭 써야하나요? 그래야 한다면 이유가 궁금합니다.

vite가 필수 요구 사항이었나요?
우선 지금 구현 하신 내용으로 봐서는 create react app을 사용하신 것 같은데 cra로 생성한 앱에는 번들러가 내장되어있기 때문에 따로 vite같은 번들러를 붙일 필요는 없습니다.

그래서 번들러의 개념은 알아두시긴 해야합니다.
webpack, vite 같은 아이들에 대해 학습해보시면 좋을 듯해요

Comment thread src/components/Card.tsx
const formattedDate = formatDateString(data.created_at);
const [imgNull, setImgNull] = useState("");
const createdTime = calculateElapsedTime(data.created_at);
const [kebabBool, setKebabBool] = useState(false);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

더 좋은 변수명이 있을 것 같습니다.
변수명이 타입(bool)을 포함하도록 하는 건 좋은 습관이 아니에요.
정확히 어떤 역할을 하는 변수인지 잘 표현해주세요.

약간의 팁을 드리자면 boolean 타입의 변수들에 경우 is~, has~ 같은 prefix를 붙이면 의미를 나타내기 좋습니다.
kebabBool같은 경우 삭제, 폴더의 추가 등 여러 동작들을 노출할지를 결정하므로 isShowMoreActions이런 느낌으로 네이밍 해볼 수 있겠네요.

Comment thread src/components/Card.tsx
Comment on lines +54 to +58
useEffect(setImageNull, [data.image_source]);
useEffect(() => {
const ago = calculateAgo(createdTime);
setAgo(ago);
}, [createdTime]);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useEffect에 넘기는 콜백을 따로 선언하는 경우도 있고 화살표함수로 바로 선언하는 경우도 있네요.
특별한 이유가 없다면 일관된 컨벤션으로 작성해주시면 좋을 듯 합니다.

Comment thread src/components/Folder.tsx
Comment on lines +18 to +20
const [selectSortName, setSelectSortName] = useState<number>(0);
const [foldLinkTitle, setFoldLinkTitle] = useState<string>("전체");
const [sortData, setSortData] = useState<FolderData[]>([]);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금도 좋지만 타입이 충분히 추론되는 경우에는 굳이 타입을 따로 적어주지 않아도 괜찮습니다.
useState에 0을 넘기면 해당 state가 자연스럽게 number 타입으로 추론되어요.
L20 처럼 단순히 배열 리터럴([]) 만으로 FolderData의 배열임이 추론되지 않는 경우에는 제네릭을 넣어주는 게 의미 있습니다.

Suggested change
const [selectSortName, setSelectSortName] = useState<number>(0);
const [foldLinkTitle, setFoldLinkTitle] = useState<string>("전체");
const [sortData, setSortData] = useState<FolderData[]>([]);
const [selectSortName, setSelectSortName] = useState(0);
const [foldLinkTitle, setFoldLinkTitle] = useState("전체");
const [sortData, setSortData] = useState<FolderData[]>([]);

Comment thread src/components/Folder.tsx
const [modalContent, setModalContent] = useState<string | null>(null);
const [modalOpen, setModalOpen] = useState(false);

const clickSortName: React.MouseEventHandler<HTMLButtonElement> = (event) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이벤트 핸들러 타입도 잘 적어주셨네요👍

Comment on lines +48 to +52
style={{
backgroundColor:
selectSortName === item.id ? "#6D6AFE" : "#fff",
color: selectSortName === item.id ? "#fff" : "black",
}}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inline style 보다는 sytled로 작성해주세요

Comment on lines +20 to +29
<S.ModalBlur>
<S.Container>
<S.CloseClick onClick={handleCloseModal} />
<S.Feature>이름 변경하기</S.Feature>
<S.ContentInput />
<S.Button>
<p>변경하기</p>
</S.Button>
</S.Container>
</S.ModalBlur>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

각 컴포넌트마다 공통되는 부분은 따로 분리해도 될 것 같아요.

function Modal({ onClose, children }) {
  return (
    <S.ModalBlur>
      <S.Container>
        { children }
      </S.Container>
    </S.ModalBlur>
  )
}

대강 이런느낌으로 분리될 것 같아요

@Tanney-102
Copy link
Copy Markdown
Collaborator

컨플릭 한 번만 해소 부탁 드립니다~!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성 죄송합니다...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants